-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add simple multi-monitor support #4178
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ohif-platform-docs canceled.
|
✅ Deploy Preview for ohif-dev canceled.
|
You can then navigate either window from the 'all' studies in the study browser tab. Study navigation currently refreshes the page, which loses the position on that page, plus any markup/annotations you have created. This is a temporary issue while the navigation is updated to use internal navigation to preserve data. |
Viewers Run #4613
Run Properties:
|
Project |
Viewers
|
Branch Review |
feat/multi-monitor-take2
|
Run status |
Passed #4613
|
Run duration | 02m 05s |
Commit |
f5e8873294: Add the simplest multi-monitor display
|
Committer | Bill Wallace |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
2
|
Skipped |
0
|
Passing |
44
|
View all changes introduced in this branch ↗︎ |
Means that the study change doesn't have to occur by updating the URL, which clears all the internal data.
} | ||
|
||
public run(screenDelta = 1, commands, options) { | ||
const screenNumber = (this.screenNumber + (screenDelta ?? 1)) % this.numberOfScreens; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using negative delta to run commands from window 2 into window 1 feels a bit weird I think. I could see explicit screen id being more intuitive. Or what's the use case with delta?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case is really left/right windows or "other window" for the two window use case.
this.isMultimonitor = false; | ||
} | ||
|
||
public run(screenDelta = 1, commands, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts about a function that would run commands and options in all windows except the origin from which it was called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, future enhancement. This is baby steps to get started.
Context
The ability to use more monitor space for viewing studies is quite useful for being able to compare current/prior or comparing various series.
Changes & Results
Added some basic navigation controls to the study browser (thumbnails list) to navigate the study to the specified one.
Testing
Launch the localhost url, OR an https URL with ?multimonitor=split or ?multimonitor=2
(use 2 only if you have two physical monitors)
Display a study which has several studies for the same MRN
Launch the study in basic test mode or in viewer mode
Right click the study title in the study browser to launch studies in the other window.
There aren't navigation settings for the launch window.
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment